Manage local Goose schedules from Berd - #245
Conversation
4851df0 to
4731727
Compare
Co-authored-by: Goose <opensource@block.xyz>
4731727 to
8051b23
Compare
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES: the client-side remove transaction can resume a schedule after another actor has paused it, and its rollback can hide the original deletion failure. Supplied GitHub checks completed successfully. Product and authorization questions require Morgan's decision.
Deterministic publication result: 2 blocking and 1 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.
| await client.goose.GooseUnstableSchedulesDelete({ scheduleId }); | ||
| } catch (error) { | ||
| if (pausedForRemoval) { | ||
| await client.goose.GooseUnstableSchedulesUnpause({ scheduleId }); |
There was a problem hiding this comment.
🤖 P1 · Removal can resume paused work (blocking)
Removal reads the schedule, pauses it, attempts deletion, and unconditionally unpauses it if deletion fails. Another client can intentionally pause the schedule after this transaction begins; rollback then overwrites that newer state and resumes future runs.
User effect: After a failed removal, a schedule someone intentionally paused can start running again without their consent.
Recommended fix: Move remove-if-idle semantics to the authoritative scheduler as an atomic operation. If unavailable, re-read and verify ownership of the temporary transition before compensating, and share the lifecycle operation between UI and berdctl.
Test: Exercise a delete failure while a concurrent actor pauses the schedule after the temporary pause; verify rollback does not unpause the newer state.
| await client.goose.GooseUnstableSchedulesDelete({ scheduleId }); | ||
| } catch (error) { | ||
| if (pausedForRemoval) { | ||
| await client.goose.GooseUnstableSchedulesUnpause({ scheduleId }); |
There was a problem hiding this comment.
🤖 P1 · Rollback hides removal failure (blocking)
The delete catch block awaits unpause before rethrowing the deletion error. If unpause also fails, the rollback failure replaces the primary deletion failure, and the failed mutation does not refresh schedule state.
User effect: A failed removal can leave future runs paused while Berd reports the wrong failure and continues showing stale schedule status.
Recommended fix: Preserve the deletion error as primary, attach rollback failure as context, and refresh schedule state after a failure that may have changed pause state.
Test: Make delete and unpause fail together; assert the deletion error remains primary, rollback failure is retained, and the UI refreshes authoritative state.
| : ""} | ||
| </div> | ||
| </div> | ||
| <div className="flex items-center gap-1.5"> |
There was a problem hiding this comment.
🤖 P2 · Schedule actions can overflow (non-blocking)
Each row may show three text buttons in a flex container that cannot wrap. Longer localized labels can overflow at narrow widths or increased text size.
User effect: At compact window widths or larger text settings, schedule controls can be clipped or extend outside their panel.
Recommended fix: Allow the action group to wrap or stack at compact widths using existing responsive layout and button primitives.
Test: Add a narrow responsive component case with Spanish copy and verify every action remains within the panel and reachable.
|
How is the public vs. internal Automations experience intended to work here? My understanding is that Goose schedules can be available in the open-source build, while G2 automations are internal-only. If this is meant to support the public Automations surface, should the G2-specific content and flows be hidden there so Goose schedules work independently? If this PR is only adding schedules to the existing internal surface, the current separation seems fine for this scope. |
2 participants
Summary
berdctl schedulecommandsWhy
Berd starts a long-lived
goosed --enable-schedulerprocess, but local Goose schedules were invisible in Berd and could only be managed with a separategoose scheduleCLI process. That creates split scheduler state: an out-of-process removal can updateschedule.jsonwhile the embedded scheduler still holds the job in memory, allowing a later status write to restore the deleted job.This change routes user and agent controls through Berd's live ACP client, so they operate on the scheduler instance that actually owns and runs the jobs.
User-visible behavior
The Automations overview now has a Local schedules section. Each job shows its cron and running/paused state, with actions to:
Agents get matching commands:
Validation
just checkjust test— 6,000+ frontend tests passcargo test -p berdctl— 52 passed, 1 ignoredfmt-check,check,tauri-check, andclippyall passArchitectural laws
Read
LAWS/README.md,LAWS/AGENTS.md, andLAWS/CHAT.md. This change does not alter agent invocation or composer queue behavior. The UI and CLI operate on the same live schedule state.